Description

Problem*

Initial work towards resolving #2133. Pushing for visibility in case we want to merge this now as this does fix reporting for index out of bounds errors from ACVM, but does not fully resolve #2133. Read the summary for more details.

Summary*

I have added reporting for IndexOutOfBounds errors triggered by ACVM. These occur when there is an index out of bounds on a dynamic arrray that uses the MemoryOp ACIR opcode. The reporting essentially extends the work for unsatisfied constrain statements, but I felt a true refactor of how nargo execute handles ACVM errors can be left for another PR.

In order to report the error accurately, a couple other changes had to be made as well. When we were creating MemoryOp's to work with dynamic arrays in ACIR we were pushing to the list of opcodes directly and not attaching location information. I have switched to calling the push_opcode method when adding opcodes that deal with memory. I have also restricted the opcodes field to now be private with the goal to avoid this error in the future. Any opcodes should be included using the push_opcode method that also attaches a location.

A new dynamic_array_index test has been included under compile_failure which will now exit with this:

The new location data has also exposed the actual bug that occurs in #2133. It looks like we are not handling locations correctly during flattening. A smaller reproduced snippet of the storage proof example is provided in the dynamic_array_index. If the dynamic array is accessed with an index out of bounds later after flattening, only the then case of the array merger is shown. For example

    if z == 20 {
        x[0] = x[4];
    } else {
        x[idx] = 10;
        for i in 0..5 {
            x[idx] = x[i];
        }
    }
    // TODO(#2133): This line should be where we show the error
    // but instead it will show x[0] == x[4] as the line where there is an index out of bounds
    assert(x[idx] != 0)

This bug is also seen in the storage proof example, which will return the below output when calling nargo execute:

However, based on the smaller snippet above, it looks to be using the incorrect location, as the array access being reported is within the then case of an if statement:

        if ((i + n) as u32) < (N as u32)
        {
            out[i] = input[i+n];
        }

Documentation

• This PR requires documentation updates when merged.

  • I will submit a noir-lang/docs PR.
  • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.